Skip to content

Adding GMT Offset for RSS Block#8543

Open
SirLouen wants to merge 5 commits intoWordPress:trunkfrom
SirLouen:patch/62400
Open

Adding GMT Offset for RSS Block#8543
SirLouen wants to merge 5 commits intoWordPress:trunkfrom
SirLouen:patch/62400

Conversation

@SirLouen
Copy link
Copy Markdown
Member

This patch solves the problem that GMT Offset is not aligning correctly with the RSS Feeds in the RSS Gutenberg Block

For testing, check the instructions in the OP Post in Trac

Trac ticket: https://core.trac.wordpress.org/ticket/62400

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 19, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props sirlouen, peterwilsoncc.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests well and I can see it working as intended running my site in Melbourne (+1100) time.

I've added some notes in the tests, mainly about avoiding work that the base test suite handles already.

Comment thread src/wp-includes/blocks/rss.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
@SirLouen
Copy link
Copy Markdown
Member Author

Ok @peterwilsoncc I think I've sorted everything

Copy link
Copy Markdown
Contributor

@peterwilsoncc peterwilsoncc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed something yesterday. I've suggested a change in name to the file but otherwise this looks great.

However, the main thing I missed was this: the changes will need to go in the Gutenberg repo (index.php in the block library package) as the file is changed during an npm install. This is the cause of some of the test failures.

If you haven't created a PR in the GB repo, I'm happy to help out and transfer the PR over there.

Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
Comment thread tests/phpunit/tests/blocks/renderRssBlock.php Outdated
@SirLouen
Copy link
Copy Markdown
Member Author

@peterwilsoncc added all recommended changes + added the patch to Gutenberg repo:
WordPress/gutenberg#69645

I've been checking, and I can't see any Gutenberg Unit Tests section.

Also, I'm not sure, if the changes to src/wp-includes/blocks/rss.php should stay, should be reverted, or are nowadays irrelevant.

I'm not 100% confident yet how the integration of Gutenberg works with WP Core. All I know there is a plugin for Gutenberg, but then I also know that there is like a slightly older and more stable version, being regularly integrated into core. But I'm not sure how this is integration is being done.

@peterwilsoncc
Copy link
Copy Markdown
Contributor

I've been checking, and I can't see any Gutenberg Unit Tests section.

The tests for blocks in the Gutenberg repo are in https://github.com/WordPress/gutenberg/tree/trunk/phpunit/blocks -- the RSS block doesn't have any tests, so you'd need to create a file.

Also, I'm not sure, if the changes to src/wp-includes/blocks/rss.php should stay, should be reverted, or are nowadays irrelevant.

I'm not 100% confident yet how the integration of Gutenberg works with WP Core. All I know there is a plugin for Gutenberg, but then I also know that there is like a slightly older and more stable version, being regularly integrated into core. But I'm not sure how this is integration is being done.

It's a little confusing.

The edited file in this pull request is actually published in the Gutenberg NPM package @wordpress/block-library and updated as part of the syncing script, npm run sync-gutenberg-packages. So once the PR is merged in to the Gutenberg repo, this PR can be discarded as the fix will be included in the appropriate syncing.

I know it's odd to have PHP in an NPM package, but needs must.

The Gutenberg plugin is the bleeding-edge work, where as the files synced in to WordPress-Develop are considered stable.

Again, sorry I missed this was a Gutenberg generated file when I reviewed it yesterday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants